From aa3005f5243a708bf9117f336863fab02e610c21 Mon Sep 17 00:00:00 2001 From: Jeffrey Yasskin Date: Wed, 16 Sep 2015 20:37:45 -0700 Subject: [PATCH] Make Dependency cheap to copy by having it store a refcounted inner object. --- src/cargo/core/dependency.rs | 97 ++++++++++++++++++++++++++++------ src/cargo/core/mod.rs | 2 +- src/cargo/core/resolver/mod.rs | 12 ++--- src/cargo/ops/cargo_package.rs | 2 +- src/cargo/sources/registry.rs | 9 ++-- src/cargo/util/toml.rs | 16 +++--- tests/resolve.rs | 11 ++-- 7 files changed, 110 insertions(+), 39 deletions(-) diff --git a/src/cargo/core/dependency.rs b/src/cargo/core/dependency.rs index 651d5f3d0..aa74490fe 100644 --- a/src/cargo/core/dependency.rs +++ b/src/cargo/core/dependency.rs @@ -1,11 +1,12 @@ use semver::VersionReq; use core::{SourceId, Summary, PackageId}; +use std::rc::Rc; use util::CargoResult; -/// Informations about a dependency requested by a Cargo manifest. +/// The data underlying a Dependency. #[derive(PartialEq,Clone,Debug)] -pub struct Dependency { +pub struct DependencyInner { name: String, source_id: SourceId, req: VersionReq, @@ -22,6 +23,13 @@ pub struct Dependency { only_for_platform: Option, } +/// Information about a dependency requested by a Cargo manifest. +/// Cheap to copy. +#[derive(PartialEq,Clone,Debug)] +pub struct Dependency { + inner: Rc, +} + #[derive(PartialEq, Clone, Debug, Copy)] pub enum Kind { Normal, @@ -29,26 +37,26 @@ pub enum Kind { Build, } -impl Dependency { +impl DependencyInner { /// Attempt to create a `Dependency` from an entry in the manifest. pub fn parse(name: &str, version: Option<&str>, - source_id: &SourceId) -> CargoResult { + source_id: &SourceId) -> CargoResult { let version_req = match version { Some(v) => try!(VersionReq::parse(v)), None => VersionReq::any() }; - Ok(Dependency { + Ok(DependencyInner { only_match_name: false, req: version_req, specified_req: version.map(|s| s.to_string()), - .. Dependency::new_override(name, source_id) + .. DependencyInner::new_override(name, source_id) }) } - pub fn new_override(name: &str, source_id: &SourceId) -> Dependency { - Dependency { + pub fn new_override(name: &str, source_id: &SourceId) -> DependencyInner { + DependencyInner { name: name.to_string(), source_id: source_id.clone(), req: VersionReq::any(), @@ -76,49 +84,49 @@ impl Dependency { self.only_for_platform.as_ref().map(|s| &s[..]) } - pub fn set_kind(mut self, kind: Kind) -> Dependency { + pub fn set_kind(mut self, kind: Kind) -> DependencyInner { self.kind = kind; self } /// Sets the list of features requested for the package. - pub fn set_features(mut self, features: Vec) -> Dependency { + pub fn set_features(mut self, features: Vec) -> DependencyInner { self.features = features; self } /// Sets whether the dependency requests default features of the package. - pub fn set_default_features(mut self, default_features: bool) -> Dependency { + pub fn set_default_features(mut self, default_features: bool) -> DependencyInner { self.default_features = default_features; self } /// Sets whether the dependency is optional. - pub fn set_optional(mut self, optional: bool) -> Dependency { + pub fn set_optional(mut self, optional: bool) -> DependencyInner { self.optional = optional; self } /// Set the source id for this dependency - pub fn set_source_id(mut self, id: SourceId) -> Dependency { + pub fn set_source_id(mut self, id: SourceId) -> DependencyInner { self.source_id = id; self } /// Set the version requirement for this dependency - pub fn set_version_req(mut self, req: VersionReq) -> Dependency { + pub fn set_version_req(mut self, req: VersionReq) -> DependencyInner { self.req = req; self } pub fn set_only_for_platform(mut self, platform: Option) - -> Dependency { + -> DependencyInner { self.only_for_platform = platform; self } /// Lock this dependency to depending on the specified package id - pub fn lock_to(self, id: &PackageId) -> Dependency { + pub fn lock_to(self, id: &PackageId) -> DependencyInner { assert_eq!(self.source_id, *id.source_id()); assert!(self.req.matches(id.version())); self.set_version_req(VersionReq::exact(id.version())) @@ -152,6 +160,63 @@ impl Dependency { (self.only_match_name || (self.req.matches(id.version()) && &self.source_id == id.source_id())) } + + pub fn into_dependency(self) -> Dependency { + Dependency {inner: Rc::new(self)} + } +} + +impl Dependency { + /// Attempt to create a `Dependency` from an entry in the manifest. + pub fn parse(name: &str, + version: Option<&str>, + source_id: &SourceId) -> CargoResult { + DependencyInner::parse(name, version, source_id).map(|di| { + di.into_dependency() + }) + } + + pub fn new_override(name: &str, source_id: &SourceId) -> Dependency { + DependencyInner::new_override(name, source_id).into_dependency() + } + + pub fn clone_inner(&self) -> DependencyInner { (*self.inner).clone() } + + pub fn version_req(&self) -> &VersionReq { self.inner.version_req() } + pub fn name(&self) -> &str { self.inner.name() } + pub fn source_id(&self) -> &SourceId { self.inner.source_id() } + pub fn kind(&self) -> Kind { self.inner.kind() } + pub fn specified_req(&self) -> Option<&str> { self.inner.specified_req() } + + /// If none, this dependencies must be built for all platforms. + /// If some, it must only be built for the specified platform. + pub fn only_for_platform(&self) -> Option<&str> { + self.inner.only_for_platform() + } + + /// Lock this dependency to depending on the specified package id + pub fn lock_to(self, id: &PackageId) -> Dependency { + self.clone_inner().lock_to(id).into_dependency() + } + + /// Returns false if the dependency is only used to build the local package. + pub fn is_transitive(&self) -> bool { self.inner.is_transitive() } + pub fn is_build(&self) -> bool { self.inner.is_build() } + pub fn is_optional(&self) -> bool { self.inner.is_optional() } + /// Returns true if the default features of the dependency are requested. + pub fn uses_default_features(&self) -> bool { + self.inner.uses_default_features() + } + /// Returns the list of features that are requested by the dependency. + pub fn features(&self) -> &[String] { self.inner.features() } + + /// Returns true if the package (`sum`) can fulfill this dependency request. + pub fn matches(&self, sum: &Summary) -> bool { self.inner.matches(sum) } + + /// Returns true if the package (`id`) can fulfill this dependency request. + pub fn matches_id(&self, id: &PackageId) -> bool { + self.inner.matches_id(id) + } } #[derive(PartialEq,Clone,RustcEncodable)] diff --git a/src/cargo/core/mod.rs b/src/cargo/core/mod.rs index 54c733348..b0b0895f0 100644 --- a/src/cargo/core/mod.rs +++ b/src/cargo/core/mod.rs @@ -1,4 +1,4 @@ -pub use self::dependency::Dependency; +pub use self::dependency::{Dependency, DependencyInner}; pub use self::manifest::{Manifest, Target, TargetKind, Profile, LibKind, Profiles}; pub use self::package::{Package, PackageSet}; pub use self::package_id::{PackageId, Metadata}; diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index 22c0584bf..1ac67168d 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -95,7 +95,7 @@ type ResolveResult = CargoResult>>; // Information about the dependencies for a crate, a tuple of: // // (dependency info, candidates, features activated) -type DepInfo = (Rc, Vec>, Vec); +type DepInfo = (Dependency, Vec>, Vec); impl Resolve { fn new(root: PackageId) -> Resolve { @@ -275,7 +275,7 @@ struct BacktrackFrame { deps_backup: Vec, remaining_candidates: RcVecIter>, parent: Rc, - dep: Rc, + dep: Dependency, } /// Recursively activates the dependencies for `top`, in depth-first order, @@ -402,7 +402,7 @@ fn activate_deps_loop(mut cx: Context, fn find_candidate(backtrack_stack: &mut Vec, cx: &mut Context, remaining_deps: &mut Vec, parent: &mut Rc, cur: &mut usize, - dep: &mut Rc) -> Option> { + dep: &mut Dependency) -> Option> { while let Some(mut frame) = backtrack_stack.pop() { if let Some((_, candidate)) = frame.remaining_candidates.next() { *cx = frame.context_backup.clone(); @@ -475,7 +475,7 @@ fn activation_error(cx: &Context, dep.version_req()); let mut msg = msg; let all_req = semver::VersionReq::parse("*").unwrap(); - let new_dep = dep.clone().set_version_req(all_req); + let new_dep = dep.clone_inner().set_version_req(all_req).into_dependency(); let mut candidates = match registry.query(&new_dep) { Ok(candidates) => candidates, Err(e) => return e, @@ -681,7 +681,7 @@ impl Context { #[allow(deprecated)] // connect => join in 1.3 fn resolve_features(&mut self, parent: &Summary, method: &Method) - -> CargoResult, Vec)>> { + -> CargoResult)>> { let dev_deps = match *method { Method::Everything => true, Method::Required { dev_deps, .. } => dev_deps, @@ -712,7 +712,7 @@ impl Context { feature))); } } - ret.push((Rc::new(dep.clone()), base)); + ret.push((dep.clone(), base)); } // All features can only point to optional dependencies, in which case diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index bcd74f460..d607e61f5 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -172,7 +172,7 @@ fn run_verify(config: &Config, pkg: &Package, tar: &Path) let new_pkgid = try!(PackageId::new(pkg.name(), pkg.version(), &new_src)); let new_summary = pkg.summary().clone().map_dependencies(|d| { if !d.source_id().is_path() { return d } - d.set_source_id(registry.clone()) + d.clone_inner().set_source_id(registry.clone()).into_dependency() }); let mut new_manifest = pkg.manifest().clone(); new_manifest.set_summary(new_summary.override_id(new_pkgid)); diff --git a/src/cargo/sources/registry.rs b/src/cargo/sources/registry.rs index 7f97cfce9..b9754852d 100644 --- a/src/cargo/sources/registry.rs +++ b/src/cargo/sources/registry.rs @@ -172,7 +172,7 @@ use tar::Archive; use url::Url; use core::{Source, SourceId, PackageId, Package, Summary, Registry}; -use core::dependency::{Dependency, Kind}; +use core::dependency::{Dependency, DependencyInner, Kind}; use sources::{PathSource, git}; use util::{CargoResult, Config, internal, ChainError, ToUrl, human}; use util::{hex, Sha256}; @@ -430,8 +430,8 @@ impl<'cfg> RegistrySource<'cfg> { name, req, features, optional, default_features, target, kind } = dep; - let dep = try!(Dependency::parse(&name, Some(&req), - &self.source_id)); + let dep = try!(DependencyInner::parse(&name, Some(&req), + &self.source_id)); let kind = match kind.as_ref().map(|s| &s[..]).unwrap_or("") { "dev" => Kind::Development, "build" => Kind::Build, @@ -449,7 +449,8 @@ impl<'cfg> RegistrySource<'cfg> { .set_default_features(default_features) .set_features(features) .set_only_for_platform(target) - .set_kind(kind)) + .set_kind(kind) + .into_dependency()) } /// Actually perform network operations to update the registry diff --git a/src/cargo/util/toml.rs b/src/cargo/util/toml.rs index f3a16517c..7e0115e04 100644 --- a/src/cargo/util/toml.rs +++ b/src/cargo/util/toml.rs @@ -10,7 +10,8 @@ use semver; use rustc_serialize::{Decodable, Decoder}; use core::{SourceId, Profiles}; -use core::{Summary, Manifest, Target, Dependency, PackageId, GitReference}; +use core::{Summary, Manifest, Target, Dependency, DependencyInner, PackageId, + GitReference}; use core::dependency::Kind; use core::manifest::{LibKind, Profile, ManifestMetadata}; use core::package_id::Metadata; @@ -631,7 +632,7 @@ fn validate_bench_name(target: &TomlTarget) -> CargoResult<()> { fn process_dependencies(cx: &mut Context, new_deps: Option<&HashMap>, mut f: F) -> CargoResult<()> - where F: FnMut(Dependency) -> Dependency + where F: FnMut(DependencyInner) -> DependencyInner { let dependencies = match new_deps { Some(ref dependencies) => dependencies, @@ -666,14 +667,15 @@ fn process_dependencies(cx: &mut Context, } }.unwrap_or(try!(SourceId::for_central(cx.config))); - let dep = try!(Dependency::parse(&n, - details.version.as_ref() - .map(|v| &v[..]), - &new_source_id)); + let dep = try!(DependencyInner::parse(&n, + details.version.as_ref() + .map(|v| &v[..]), + &new_source_id)); let dep = f(dep) .set_features(details.features.unwrap_or(Vec::new())) .set_default_features(details.default_features.unwrap_or(true)) - .set_optional(details.optional.unwrap_or(false)); + .set_optional(details.optional.unwrap_or(false)) + .into_dependency(); cx.deps.push(dep); } diff --git a/tests/resolve.rs b/tests/resolve.rs index 99094da65..0e551ac96 100644 --- a/tests/resolve.rs +++ b/tests/resolve.rs @@ -6,7 +6,7 @@ use std::collections::HashMap; use hamcrest::{assert_that, equal_to, contains}; use cargo::core::source::{SourceId, GitReference}; -use cargo::core::dependency::Kind::Development; +use cargo::core::dependency::Kind::{self, Development}; use cargo::core::{Dependency, PackageId, Summary, Registry}; use cargo::util::{CargoResult, ToUrl}; use cargo::core::resolver::{self, Method}; @@ -106,6 +106,9 @@ fn dep_loc(name: &str, location: &str) -> Dependency { let source_id = SourceId::for_git(&url, master); Dependency::parse(name, Some("1.0.0"), &source_id).unwrap() } +fn dep_kind(name: &str, kind: Kind) -> Dependency { + dep(name).clone_inner().set_kind(kind).into_dependency() +} fn registry(pkgs: Vec) -> Vec { pkgs @@ -192,14 +195,14 @@ fn test_resolving_with_same_name() { #[test] fn test_resolving_with_dev_deps() { let mut reg = registry(vec!( - pkg!("foo" => ["bar", dep("baz").set_kind(Development)]), - pkg!("baz" => ["bat", dep("bam").set_kind(Development)]), + pkg!("foo" => ["bar", dep_kind("baz", Development)]), + pkg!("baz" => ["bat", dep_kind("bam", Development)]), pkg!("bar"), pkg!("bat") )); let res = resolve(pkg_id("root"), - vec![dep("foo"), dep("baz").set_kind(Development)], + vec![dep("foo"), dep_kind("baz", Development)], &mut reg).unwrap(); assert_that(&res, contains(names(&["root", "foo", "bar", "baz"]))); -- 2.30.2